Conversation
g-saracca
added
Size: 3
g-saracca
changed the title
ekraffmiller
left a comment
ekraffmiller
left a comment
There was a problem hiding this comment.
Looks good! Just a couple of questions
| * @param {boolean} [restrict] - A boolean value that indicates whether the file should be restricted or unrestricted. | ||
| * @returns {Promise<void>} -This method does not return anything upon successful completion. | ||
| */ | ||
| async execute(fileId: number | string, restrict: boolean): Promise<void> { |
There was a problem hiding this comment.
In the UI, the restrict file modal includes an option of changing the default fileAccessRequest value, and adding termsOfAccessForRestrictedFiles. (these values are also in the Terms tab). Do we want to add this as optional params to the restrict method? Or from the frontend, should it be handled by making a separate API call to update Dataset Terms of Use?
There was a problem hiding this comment.
Good question, the API is not accepting params and I think it's ok.
A second API call to update Dataset Terms of Use seems reasonable.
We can do that separately while working on the frontend integration. Is there a use case for doing that in Js-dataverse?
There was a problem hiding this comment.
ok, we don't have a use case for updating terms of use yet, so yes would be good to work on that separately.
|
@ekraffmiller ready for review again. |
ekraffmiller
left a comment
ekraffmiller
left a comment
There was a problem hiding this comment.
Looks good, approved!
|
tests are passing - merging pr |
3 participants
What this PR does / why we need it:
Adds the Restrict File use case. It can restrict or unrestrict a file.
Which issue(s) this PR closes:
Suggestions on how to test this:
Review code and tests.